feat: support crossplane 2.3 api types#128
Conversation
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
|
Hey there, Yes, the angryjet generation of the usual methodsets for 2.3 resources is currently broken. |
adamwg
left a comment
There was a problem hiding this comment.
I'm new to this codebase and not a maintainer, but this looks correct to me.
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
|
@negz I added tests for angryjet |
jbw976
left a comment
There was a problem hiding this comment.
Thanks again for driving this change @floric - a lot of the folks in the community are going to benefit from it, so we appreciate your initiative and follow up on getting this merged! 🙇♂️
I'm curious about the ProviderConfig* types needing to be updated too 🤔
| } | ||
|
|
||
| // GenerateProviderConfig generates the resource.ProviderConfig method set. | ||
| func GenerateProviderConfig(filename, header string, p *packages.Package) error { |
There was a problem hiding this comment.
Does the ProviderConfig* support also need to get updated like the MR support is? It looks like the ProviderConfig, ProviderConfigUsage, and ProviderConfigUsageList matchers still key on the old common/v1 / common/v2 types, so I don't think they'd match a provider whose PC/PCU types embed the new core/v2 equivalents.
What do you think?
There was a problem hiding this comment.
yes, you are absolutely right. I added more types. I didn't test it again for our example as this one is already migrated. Maybe somebody has more examples for testing?
Signed-off-by: Florian Fa Richter <florian.fa.richter@deutschebahn.com>
Adds the github.com/crossplane/crossplane/apis/v2 module and a temporary replace directive pointing crossplane-tools at the floric/crossplane-tools fork which is under test in crossplane/crossplane-tools#128. That PR updates angryget method set generation for the new modules/names location of crossplane APIs and core v2 types. Also bumps Go 1.24 to 1.25, golangci-lint to 2.12.2, the Kubernetes libraries to 0.35, controller-runtime to 0.23.1, controller-tools to 0.20.0, and the transitive dependency tail. Signed-off-by: Jared Watts <jbw976@gmail.com>
|
I've been putting this through a bit of testing with the The one thing I'm still thinking through though is the naming and consistency across the all the types. Between the tier names (Legacy / Modern / V2) and the three import sources (crossplane-runtime I don't think any of it blocks the core functionality, but I'm going to think a bit more deeply on if there are some naming updates we should make here too so everything stays nice and clear as more providers pick up the core v2 types. I'll get back with a suggestion ASAP! 🤔 🏃 |
jbw976
left a comment
There was a problem hiding this comment.
OK I got to think through the naming and consistency a bit more here. This PR is really about solving the breaking v2.3 change where the common API types moved out of crossplane-runtime (apis/common/*) and into the crossplane/apis/v2/core/v2 module. So what a v2.3+ based provider needs from angryjet is the same method sets it already generates, but just with the types imported from the new module location.
That means we should be able to keep this purely additive. Rather than renaming or repointing the existing generators, let's leave them as they are (i.e. revert any changes to existing stuff) and add a parallel set that mirrors each shape but imports from crossplane/apis/v2/core/v2, using a Core suffix to indicate it's types are coming from the new module. For example:
GenerateManagedLegacyGenerateManagedLegacyCoreGenerateManagedModernGenerateManagedModernCore- etc.
Each *Core twin would be the same generated method set as its non-Core sibling,
just using the new core APIs module instead of the old crossplane-runtime one where needed.
What's thrown me off a bit so far with the current naming in this PR of *Namespaced and *Cluster is that we're using those terms to kinda mean the same thing as *Legacy and *Modern while they are actually proxy names for the API module changing.
What I like about consistently using Modern/Legacy is that they will each mean only one thing across every kind, and Core would be the only new concept introduced in this PR.
What do you think of that approach? If that's reasonable, then let's do the following:
- Update the naming of the methods in this PR (note a couple of them aren't yet covered so would need to be added instead of just renamed):
- GenerateManagedLegacyCore
- GenerateManagedModernCore
- GenerateManagedListLegacyCore
- GenerateManagedListModernCore
- GenerateProviderConfigCore
- GenerateProviderConfigUsageLegacyCore (currently missing)
- GenerateProviderConfigUsageModernCore
- GenerateProviderConfigUsageListLegacyCore (currently missing)
- GenerateProviderConfigUsageListModernCore
- GenerateReferencesLegacyCore
- GenerateReferencesModernCore
- Update the naming of the matchers / add new matchers similarly
- Updates that were made to existing generators/matchers should be reverted, I think those can all stay untouched from how they are in
mainand our additive ones will cover the new scenarios we care about - Note that
ProviderConfigis the one exception with noLegacy/Modernsplit (justProviderConfigandProviderConfigCore), since its generated set doesn't vary by scope the way managed and PCU do.
What do you think? I really appreciate your time spent on this and thanks for working with me to help make this easily understandable for future contributors as well. I'm going to keep dedicating time to reviewing/testing this so we can get this merged and your effort will have a pay-off for the community. Thank you! 🙇♂️
Description of your changes
Add compatibility to Crossplane 2.3 by supporting new types and keeping backwards compatibility.
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
Tested on provider-gitlab.